Skip to content

Implement bin sort with minsigma - #196

Merged
y0z merged 2 commits into
optuna:mainfrom
not522:bin-sort
Aug 19, 2026
Merged

Implement bin sort with minsigma#196
y0z merged 2 commits into
optuna:mainfrom
not522:bin-sort

Conversation

@not522

@not522 not522 commented Aug 12, 2026

Copy link
Copy Markdown
Member

The sorting process for calculating bandwidth is consuming significant computation time. By distributing observations into bins of width minsigma, I have simplified and accelerated this sorting operation.

Benchmark

  • master
n_params  n_trials    total [ms]  per trial [us]
      40       500         410.3         820.5
      40      1000        1457.6        1457.6
      40      2000        5691.5        2845.7
  • PR
n_params  n_trials    total [ms]  per trial [us]
      40       500         383.9         767.8
      40      1000        1287.8        1287.8
      40      2000        4898.1        2449.1
Details
use std::hint::black_box;
use std::time::{Duration, Instant};

use rustuna_core::storage::InMemoryStorage;
use rustuna_core::study::{create_study, Direction};
use rustuna_core::Result;
use rustuna_sampler::tpe::TpeSampler;

const N_TRIALS: [usize; 3] = [500, 1000, 2000];
const N_PARAMS: [usize; 1] = [40];

fn run_study(n_trials: usize, n_params: usize) -> Result<Duration> {
    let storage = InMemoryStorage::new();
    let study = create_study(
        "tpe-benchmark",
        storage,
        TpeSampler::seed_from_u64(0),
        vec![Direction::Minimize],
    )?;

    let start = Instant::now();
    study.optimize(
        |mut trial| {
            let mut value = 0.0;
            for i in 0..n_params {
                let x = trial.suggest_float(&format!("x{i}"), -10.0, 10.0)?;
                value += x * x;
            }
            Ok(vec![black_box(value)])
        },
        n_trials,
    )?;
    Ok(start.elapsed())
}

fn main() -> Result<()> {
    println!(
        "{:>8}  {:>8}  {:>12}  {:>12}",
        "n_params", "n_trials", "total [ms]", "per trial [us]"
    );
    for n_params in N_PARAMS {
        for n_trials in N_TRIALS {
            let duration = run_study(n_trials, n_params)?;
            println!(
                "{:>8}  {:>8}  {:>12.1}  {:>12.1}",
                n_params,
                n_trials,
                duration.as_secs_f64() * 1e3,
                duration.as_secs_f64() * 1e6 / n_trials as f64,
            );
        }
    }
    Ok(())
}

@c-bata

c-bata commented Aug 13, 2026

Copy link
Copy Markdown
Member

@y0z Could you review this PR?

Comment on lines +240 to +251
@@ -209,6 +246,9 @@ impl NumericalDistributionBuilder for DefaultNumericalDistributionBuilder {
}
}

// Sigma for prior
sigmas.push(adj_high - adj_low);

@y0z y0z Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving sigmas.push here from above decreases sigmas.len() when computing minsigma (Using 2.0 + sigmas.len() may resolve the issue?).

(before)

  ...
  sigmas.push(...)
  let minsigma = (adj_high - adj_low) / (100.0_f64.min(1.0 + sigmas.len() as f64));
}

(after)

  ...
  let minsigma = (adj_high - adj_low) / (100.0_f64.min(1.0 + sigmas.len() as f64));
}
sigmas.push(...)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing it out. I fixed the bug by using the original code as much as possible.

@y0z y0z left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@y0z
y0z merged commit 65cafcf into optuna:main Aug 19, 2026
7 checks passed
@not522
not522 deleted the bin-sort branch August 19, 2026 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants